Skip to content

fix(send): skip IMAP APPEND when server already saved sent message#12932

Open
ChristophWurst wants to merge 2 commits into
mainfrom
fix/avoid-duplicate-sent
Open

fix(send): skip IMAP APPEND when server already saved sent message#12932
ChristophWurst wants to merge 2 commits into
mainfrom
fix/avoid-duplicate-sent

Conversation

@ChristophWurst
Copy link
Copy Markdown
Member

@ChristophWurst ChristophWurst commented May 13, 2026

Some mail servers (e.g. Microsoft Exchange/Office 365) automatically save a copy of sent messages to the Sent folder upon SMTP submission. When Nextcloud Mail also does an IMAP APPEND, the user ends up with duplicates.

After SMTP delivery succeeds, search the Sent mailbox for the outgoing message's Message-ID header. If the server already saved it, skip the APPEND. If the IMAP search fails for any reason, fall through and APPEND as normal (safe degradation).

AI-assisted: Claude Code (claude-sonnet-4-6)

Summary by CodeRabbit

  • Bug Fixes

    • Prevented duplicate saves to the Sent folder when the mail server has already stored the sent message (handler now skips redundant appends).
  • Tests

    • Added unit and integration tests to verify skipping behavior and proper fallback when mailbox existence checks fail.

Review Change Stack

@ChristophWurst
Copy link
Copy Markdown
Member Author

/backport to stable5.8

@ChristophWurst
Copy link
Copy Markdown
Member Author

/backport to stable5.7

Some mail servers (e.g. Microsoft Exchange/Office 365) automatically save
a copy of sent messages to the Sent folder upon SMTP submission. When
Nextcloud Mail also does an IMAP APPEND, the user ends up with duplicates.

After SMTP delivery succeeds, search the Sent mailbox for the outgoing
message's Message-ID header. If the server already saved it, skip the
APPEND. If the IMAP search fails for any reason, fall through and APPEND
as normal (safe degradation).

AI-assisted: Claude Code (claude-sonnet-4-6)
Signed-off-by: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com>
@ChristophWurst ChristophWurst force-pushed the fix/avoid-duplicate-sent branch from 63b0914 to 09a5b4c Compare May 13, 2026 09:31
@ChristophWurst ChristophWurst marked this pull request as ready for review May 13, 2026 10:07
@ChristophWurst
Copy link
Copy Markdown
Member Author

@CodeRabbit review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e6a1d310-11ca-47a6-b9b5-ff5ab967d2a8

📥 Commits

Reviewing files that changed from the base of the PR and between 09a5b4c and 8475dea.

📒 Files selected for processing (1)
  • lib/Send/CopySentMessageHandler.php

📝 Walkthrough

Walkthrough

Adds Message-ID-based duplicate detection: MessageMapper gains existsInMailboxByMessageId(...), CopySentMessageHandler uses it to skip APPEND when the Sent mailbox already contains the message, and unit + integration tests cover skip and fallback paths.

Changes

Skip Append When Server Already Saves Message

Layer / File(s) Summary
Message-ID existence lookup in MessageMapper
lib/IMAP/MessageMapper.php
New public method existsInMailboxByMessageId() performs IMAP SEARCH for messages matching a Message-ID header and returns whether any matching message exists in the mailbox.
CopySentMessageHandler server-save detection and skip
lib/Send/CopySentMessageHandler.php
Imports OCA\Mail\Db\Mailbox, adds private helper isAlreadySavedByServer() to extract Message-ID and consult MessageMapper, and updates process() to mark message STATUS_PROCESSED and skip append when the server already saved it.
Unit tests for skip and fallback paths
tests/Unit/Send/CopySendMessageHandlerTest.php
Adds tests that verify the handler skips append when Message-ID exists and falls back to append when the IMAP search throws, asserting status changes and next-handler invocation.
Integration test for server pre-save scenario
tests/Integration/Service/MailTransmissionIntegrationTest.php
End-to-end test simulates a pre-saved Sent message with a generated Message-ID, invokes the handler, and asserts the Sent mailbox remains with a single message (no duplicate append).
sequenceDiagram
  participant Handler as CopySentMessageHandler
  participant Helper as isAlreadySavedByServer
  participant Mapper as MessageMapper
  participant IMAP as IMAP Server
  Handler->>Helper: extract Message-ID, check server state
  Helper->>Mapper: existsInMailboxByMessageId(messageId, mailbox)
  Mapper->>IMAP: SEARCH HEADER Message-ID "messageId"
  IMAP-->>Mapper: search results (count)
  Mapper-->>Helper: exists? (true/false)
  alt Server already saved
    Helper-->>Handler: true
    Handler->>Handler: set STATUS_PROCESSED, skip append
  else Need to append
    Helper-->>Handler: false
    Handler->>Handler: proceed with normal save flow
  end
Loading

🎯 3 (Moderate) | ⏱️ ~20 minutes

🐰 A tiny hop, a message id,
The server saved it, swift and sly,
Mapper peeks and gives a nod,
Handler skips the extra plod,
Sent folder stays neat — hop bye-bye! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and specifically describes the main change: adding logic to skip IMAP APPEND when a mail server has already automatically saved the sent message, which is the core problem being solved across the modified files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/avoid-duplicate-sent

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/Send/CopySentMessageHandler.php`:
- Around line 33-35: The Message-ID regex currently inspects the entire MIME
payload ($rawMessage) in CopySentMessageHandler, which can match a
Message-ID-like line in the body; instead extract the header block only
(everything before the first blank line) and run the Message-ID match against
that header string. Locate the code using $rawMessage (the block with
preg_match('/^Message-ID:.../im', $rawMessage, $m)), split or capture headers
(e.g. split on the first double newline using preg_split or preg_match with
'/\A(.*?)(?:\r?\n){2}/s') to get $headers, then apply the existing
'/^Message-ID:\s*(<[^>]+>)/im' regex to $headers and return false if not found.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a106c86d-862d-4aca-9cb9-d638dc3622ed

📥 Commits

Reviewing files that changed from the base of the PR and between 8cb01dc and 09a5b4c.

📒 Files selected for processing (4)
  • lib/IMAP/MessageMapper.php
  • lib/Send/CopySentMessageHandler.php
  • tests/Integration/Service/MailTransmissionIntegrationTest.php
  • tests/Unit/Send/CopySendMessageHandlerTest.php

Comment thread lib/Send/CopySentMessageHandler.php Outdated
…sage

Signed-off-by: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com>
@ChristophWurst
Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: 🏗️ In progress

Development

Successfully merging this pull request may close these issues.

1 participant